-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix monitor parse and alerts table #645
Conversation
Signed-off-by: Ashish Agrawal <ashisagr@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you tag the commit where alert was removed in the PR description?
@@ -22,7 +22,7 @@ const getMonitors = async (params) => { | |||
|
|||
const parseMonitor = (monitor) => { | |||
const state = monitor.monitor.enabled ? 'enabled' : 'disabled'; | |||
const latestAlert = monitor.latestAlert === "--" ? undefined : monitor.latestAlert; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there tests for parseMonitor
? At least should be one added for this scenario
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will follow up on tests and will create a tracking issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codecov Report
@@ Coverage Diff @@
## main #645 +/- ##
=======================================
Coverage 49.99% 49.99%
=======================================
Files 231 231
Lines 6509 6509
Branches 927 927
=======================================
Hits 3254 3254
Misses 3252 3252
Partials 3 3 |
Signed-off-by: Ashish Agrawal <ashisagr@amazon.com> (cherry picked from commit 6972667)
Signed-off-by: Ashish Agrawal <ashisagr@amazon.com> (cherry picked from commit 6972667)
Description
Fix retrieving the correct latest alert time for the Alerts on Dashboards visualization feature.
Fix showing the alerts table on monitor details page. The issue was introduced in #611
Issues Resolved
N/A
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.